Skip to content

Fix LaTeX math nesting, margins persistence, pdfBlob URL leak, and CSS variable references#39

Merged
impxcts merged 14 commits intomainfrom
columnfix
Apr 9, 2026
Merged

Fix LaTeX math nesting, margins persistence, pdfBlob URL leak, and CSS variable references#39
impxcts merged 14 commits intomainfrom
columnfix

Conversation

@bdtran2002
Copy link
Copy Markdown
Contributor

@bdtran2002 bdtran2002 commented Apr 8, 2026

Several correctness and resource management bugs introduced alongside the layout customization features. Fixes span backend LaTeX generation, frontend state persistence, and CSS.

Backend

  • Nested math mode (latex_utils.py): \[ \adjustbox{...}{$\displaystyle ...$} \] is invalid — \[ already opens display-math. Removed inner $...$:
    % Before (broken)
    \[ \adjustbox{max width=\linewidth}{$\displaystyle <formula>$} \]
    % After
    \[ \adjustbox{max width=\linewidth}{\displaystyle <formula>} \]
  • extarticle for small font sizes: article class doesn't accept 8pt/9pt; switch to extarticle for those sizes.
  • Weak spacing fallback test: test_generate_sheet_invalid_spacing_defaults only checked titlespacing exists (true for all presets). Now asserts specific large preset values (16pt/8pt).

Frontend

  • margins not persisted (CreateCheatSheet.jsx): handleSave and handleClear both omitted margins from the onSave payload, silently dropping the user's margin choice on save/restore and resetting to a stale value on clear.
  • clearLatex() missing margins reset (latex.js): In-memory margins state was not reset alongside columns/fontSize/spacing, diverging from the cleared localStorage state. Added setMargins('0.25in').
  • pdfBlob URL leak (latex.js): Each compile called URL.createObjectURL() without revoking the previous URL. Added pdfBlobUrlRef to track the active URL; old URLs are revoked before overwriting, and a cleanup useEffect revokes on unmount.
  • Undefined CSS variables (App.css): .btn.history-btn referenced --surface and --text-primary, which are not defined in :root or the light theme. Replaced with the actual tokens --box-bg and --text.

- Backend: Dynamic LaTeX header/footer generation with columns/font_size/margins support
- Backend: generate_sheet endpoint now accepts layout options from request
- Frontend: LayoutOptions component with column and text size dropdowns
- Frontend: Compile button regenerates LaTeX with current layout options
- All 26 backend tests pass
Add \raggedcolumns after \begin{multicols} to prevent content bleeding
between columns when using 3-column layout.
- Add spacing dropdown (tiny/small/medium/large) controlling vertical spacing
- Reduce section/subsection header sizes via titleformat
- Center editor layout with 42.5%/42.5% pane widths and 2.5% margins
…ontrol, and spacing improvements

- Middle compile button now preserves user edits (uses handleCompileOnly)
- Add whitelist validation for font_size, margins, spacing in backend API
- Add 8 new API tests for layout parameters and injection blocking
- Fix initial-load precedence for empty string content
- Debounce localStorage auto-save (500ms)
- Add margins dropdown to UI with 4 options
- Make spacing values more drastic (tiny=0pt to large=16pt)
- User text color changed to blue in editor
- Add setTimeout/clearTimeout to eslint globals
- Generated code uses default text color
- User-typed text shows as bright white (#ffffff)
- Uses contentModified state to track user changes
- Resets to default color when regenerating from formulas
- Remove unused setContent variable
- Fix handlePreview dependency array to include margins and saveToHistory
- Update README: font size 8pt-12pt (matches UI)
Copilot AI review requested due to automatic review settings April 8, 2026 20:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the cheat sheet generator’s customization and robustness by adding layout controls (columns/font size/spacing/margins), persisting user state in localStorage, and updating the LaTeX generation pipeline to emit option-driven LaTeX.

Changes:

  • Frontend: adds layout controls + editor/preview UI updates, plus autosave/version history behavior via localStorage-backed hooks.
  • Backend: adds validated layout parameters to /api/generate-sheet/ and refactors LaTeX generation to build dynamic headers/footers and auto-scale formulas.
  • Docs/tests: updates README and expands API tests for new layout params.

Reviewed changes

Copilot reviewed 16 out of 21 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
README.md Documents new UI/features and backend utility file; minor setup snippet tweak.
frontend/src/index.css Expands root container to full-width layout.
frontend/src/hooks/latex.js Adds autosave, compile-only, layout option state, and version history.
frontend/src/hooks/formulas.js Persists class/category/formula selections to localStorage.
frontend/src/components/CreateCheatSheet.jsx Introduces line-numbered editor UI, history controls, and layout option selectors.
frontend/src/App.jsx Persists current sheet to localStorage and adjusts header padding.
frontend/src/App.css Major UI restyle + responsive layout changes for split-pane editor/preview.
frontend/eslint.config.js Adds timer globals to ESLint environment.
backend/api/views.py Adds layout param whitelist validation and passes options into LaTeX generation.
backend/api/tests.py Adds tests for new /api/generate-sheet/ layout parameters and sanitization.
backend/api/latex_utils.py Implements dynamic header/footer generation and formula auto-scaling.
.playwright-mcp/*.yml Updates recorded UI snapshots for new controls/layout.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +45 to +55
def build_dynamic_header(columns=2, font_size="10pt", margins="0.25in", spacing="large"):
"""
Build a dynamic LaTeX header based on user-selected options.
"""
size_command = FONT_SIZE_MAP.get(font_size, "\\footnotesize")
sec_before, sec_after, subsec_before, subsec_after, _ = SPACING_MAP.get(spacing, SPACING_MAP["large"])

header_lines = [
f"\\documentclass[{font_size},fleqn]{{article}}",
f"\\usepackage[margin={margins}]{{geometry}}",
"\\usepackage{amsmath, amssymb}",
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build_dynamic_header() injects font_size into \\documentclass[...] even for 8pt/9pt. The standard article class doesn’t support 8pt/9pt options, which can cause compilation failures (especially in the /api/compile/ path). Consider using extarticle (or extsizes) when font_size is 8pt/9pt, or keep the documentclass at 10pt/11pt/12pt and rely on the size command (\tiny/\scriptsize/...) for density.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in commit dd4a46c. build_dynamic_header() now uses extarticle (from the extsizes package) when font_size is 8pt or 9pt, and the standard article class for 10pt/11pt/12pt. Added three new tests to verify the correct document class is chosen for each case.

Comment on lines 146 to +150
# Escape special LaTeX characters in the formula name
escaped_name = name.replace("\\", "\\textbackslash ").replace("&", "\\&").replace("%", "\\%").replace("#", "\\#").replace("_", "\\_").replace("^", "\\textasciicircum ").replace("{", "\\{").replace("}", "\\}")
body_lines.append("\\textbf{" + escaped_name + "}")
body_lines.append("\\[ " + latex + " \\]")
body_lines.append("\\\\[4pt]")
body_lines.append("\\[ \\adjustbox{max width=\\linewidth}{$\\displaystyle " + latex + "$} \\]")
body_lines.append(f"\\\\[{formula_gap}]")
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated formula block nests math mode: \\[ ... $\\displaystyle ...$ ... \\]. Since \\[ already enters display-math, the inner $...$ will break LaTeX compilation. Wrap the formula in a single math environment (either remove \\[\\] and keep $...$ inside \\adjustbox{...}{...}, or keep \\[\\] and remove the inner $...$).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 674697e. Removed the inner $...$ from the formula adjustbox block — \[ already enters display-math mode, so the wrapper is now \[ \adjustbox{max width=\linewidth}{\displaystyle <latex>} \].

Comment on lines 558 to 561
const handleSave = (e) => {
e.preventDefault();
onSave({ title, content });
onSave({ title, content, columns, fontSize, spacing });
};
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleSave persists columns/fontSize/spacing but not margins, even though margins are user-configurable and are used for sheet generation. This will drop the user’s margin choice when restoring from currentCheatSheet.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 674697e. Added margins to handleSave and set it to '0.25in' in handleClear, matching the hook default.

Comment on lines 563 to 568
const handleClear = () => {
if (window.confirm('Are you sure you want to clear everything? This cannot be undone.')) {
clearLatex();
clearSelections();
onSave({ title: '', content: '' }, false);
onSave({ title: '', content: '', columns: 2, fontSize: '10pt', spacing: 'large' }, false);
}
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleClear calls onSave(...) without a margins field, so after clearing, currentCheatSheet may retain an old margin value (and it won’t match the reset UI defaults). Include margins (and ensure defaults match the hook defaults).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was already addressed in commit 674697e. handleClear now passes margins: '0.25in' to onSave, matching the hook default, and handleSave also includes margins to persist the user's choice.

Comment on lines 297 to 306
const clearLatex = () => {
setTitle('');
setContent('');
setColumns(2);
setFontSize('10pt');
setSpacing('large');
setPdfBlob(null);
setCompileError(null);
localStorage.removeItem(STORAGE_KEY);
};
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clearLatex() resets columns/fontSize/spacing but does not reset margins, and it also removes the storage key. This can leave the in-memory margins state out of sync with the cleared persisted state. Reset margins to the default along with the other layout options.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 674697e. Added setMargins('0.25in') to clearLatex() so in-memory state stays in sync with the cleared persisted state. Also revokes the pdfBlob URL in clearLatex() as part of the same cleanup.

Comment on lines +141 to +145
throw new Error(errorMsg);
}
const blob = await response.blob();
setPdfBlob(URL.createObjectURL(blob));
} catch (error) {
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pdfBlob stores an object URL created via URL.createObjectURL(blob) but the previous URL is never revoked when a new preview is generated or when the component unmounts. This will leak memory over repeated compiles; revoke the old URL before overwriting and in a cleanup effect.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 674697e. Added pdfBlobUrlRef to track the current object URL. The old URL is now revoked before creating a new one in both handleCompileOnly and handlePreview, and a cleanup useEffect revokes it on unmount.

Comment on lines +201 to +209
.btn.history-btn {
padding: 4px 8px;
font-size: 0.875rem;
background: var(--surface);
color: var(--text-primary);
border: 1px solid var(--border);
border-radius: 4px;
cursor: pointer;
transition: all 0.15s ease;
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.btn.history-btn uses CSS variables --surface and --text-primary, but those variables aren’t defined in :root or the light theme tokens in this file (the rest of the file uses --box-bg/--text). This will cause inconsistent styling depending on browser fallback behavior.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 674697e. Replaced var(--surface) with var(--box-bg) and var(--text-primary) with var(--text), which are both defined in :root.

Comment on lines +405 to +417
def test_generate_sheet_invalid_spacing_defaults(self, api_client):
"""Invalid spacing should be replaced with default."""
resp = api_client.post(
"/api/generate-sheet/",
{
"formulas": [{"class": "ALGEBRA I", "category": "Linear Equations", "name": "Slope Formula"}],
"spacing": "huge"
},
format="json",
)
assert resp.status_code == 200
tex = resp.json()["tex_code"]
assert "titlespacing" in tex
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_generate_sheet_invalid_spacing_defaults doesn’t actually verify that an invalid spacing value falls back to the default spacing; it only asserts titlespacing exists, which will be true for all spacing presets. Assert on a specific default value (e.g., the large preset numbers) or on the absence of the invalid preset’s numbers.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 674697e. The test now asserts the specific large preset titlespacing values: \titlespacing*{\section}{0pt}{16pt}{8pt} and \titlespacing*{\subsection}{0pt}{8pt}{4pt}, confirming the fallback produces the correct default output.

Copilot AI changed the title Fix user edits being deleted on recompile, add margins UI control Use extarticle class for 8pt/9pt font sizes in LaTeX header generation Apr 9, 2026
…eak, CSS vars, test assertion

Agent-Logs-Url: https://github.com/ChicoState/cheat-sheet/sessions/d411c334-d214-44b6-b9e8-b945233ff62a

Co-authored-by: bdtran2002 <19805162+bdtran2002@users.noreply.github.com>
Copilot AI changed the title Use extarticle class for 8pt/9pt font sizes in LaTeX header generation Fix LaTeX math nesting, margins persistence, pdfBlob URL leak, and CSS variable references Apr 9, 2026
Copilot AI requested a review from Davictory2003 April 9, 2026 17:09
@impxcts impxcts merged commit 96295a4 into main Apr 9, 2026
4 checks passed
@Davictory2003 Davictory2003 deleted the columnfix branch April 9, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants